Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix User-Agent when OSDescription has unmatched parenthesis #4991

Merged
merged 2 commits into from
Jan 9, 2023

Conversation

zivkan
Copy link
Member

@zivkan zivkan commented Dec 27, 2022

Bug

Fixes: NuGet/Home#11995

Regression? Last working version:

Description

HTTP header syntax has a section/token/syntax called comment, which is surrounded by parenthesis, but the comment itself can have a nested comment. In other words, the opening and closing parenthesis must be balanced.

Since we pass RuntimeInformation.OSDescription as a comment in NuGet's User-Agent header, we need to sanitize the input (escape the parenthesis), just in case it has unbalanced parenthesis. Even when they are balanced, NuGet isn't using it to provide a comment on the rest of the string, so I believe it's valid to always escape them.

PR Checklist

  • PR has a meaningful title

  • PR has a linked issue.

  • Described changes

  • Tests

    • Automated tests added
    • OR
    • Test exception
    • OR
    • N/A
  • Documentation

    • Documentation PR or issue filled
    • OR
    • N/A

@zivkan zivkan requested a review from a team as a code owner December 27, 2022 13:48
donnie-msft
donnie-msft previously approved these changes Dec 28, 2022
{
var target = new UserAgentStringBuilder();

var result = target.WithOSDescription(osDescription).Build();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use explicit type

erdembayar
erdembayar previously approved these changes Jan 3, 2023
@zivkan zivkan dismissed stale reviews from erdembayar and donnie-msft via 468f603 January 5, 2023 09:29
@zivkan zivkan force-pushed the dev-zivkan-http-user-agent-invalid-format branch from 85dc801 to 468f603 Compare January 5, 2023 09:29
@erdembayar
Copy link
Contributor

erdembayar commented Jan 7, 2023

I requeued failed test, now it's green.

@zivkan zivkan merged commit 9a60318 into dev Jan 9, 2023
@zivkan zivkan deleted the dev-zivkan-http-user-agent-invalid-format branch January 9, 2023 08:05
jeffkl added a commit that referenced this pull request Jan 31, 2023
aortiz-msft pushed a commit that referenced this pull request Jan 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Custom kernel breaks nuget
3 participants